-
Notifications
You must be signed in to change notification settings - Fork 13
Attempt to fix mac tests #770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Attempt to fix mac tests #770
Conversation
|
Looking at this scroll test first: Linux local: Mac on CI: The block is obviously way below the viewport for some reason. Also it's interesting to see that the viewport sizes are different which suggests discrepancies in test behaviors. Lack of cross-platform or cross-environment determinism will make this more difficult. |
Attempt to use BiDi and upgrade WIO so that viewport manipulation can be used instead of window management for better compatibility.
|
Oh hey. It actually passed when using viewport instead of window size. Cool--I didn't expect that. I was guessing there could be some differences but I wasn't expecting there to be enough of one to cause that much of a coordinate discrepancy. I'm surprised but I don't care about the why quite enough to dig on it. |
|
For reference, here are the results running the changed test on each platform. Linux (local): Mac (CI): They are basically identical now which is great. :D Unfortunately other Mac failures are likely a different problem entirely. Will dig on those next. |
This test suite seems to now pass consistently between Linux & Mac.
|
Aha--enabling BiDi caused a bunch of new failures! Interesting. |
|
Updating to latest Webdriver seems to fix a few more issues with BiDi and bring us back to basically where we were at before for failures (I think), plus a new failure affecting Linux (maybe only?). Edit: Actually interestingly the previously fixed test is either failing again, or is failing when run in conjunction with other tests. |
It's failing either again (after WIO update) or when run with the rest of the test suite.
|
Either it's failing again after the upgrade or, maybe more likely, it was flaky and incidentally passed earlier. Latest dimensions with the failure: This is more or less what we saw earlier. It seems that the BiDi change is probably not needed (and we could downgrade back if I can find fixes for everything without it). The flake seems to be that, for some reason, the block is being put in the wrong place. |
|
Huh. This is surprisingly difficult to break. It seems much more consistent now, but it obviously can still fail per the two repros above. |
|
It passed 50 times in a row. Okay...this might be really tricky actually. My current working theory is that the extra browser waits to perform the debugging are actually adding a pause that allows scrolling to stabilize, thus fixing the test. |
|
The extra pause seems to fix it. Removing all debug logs successfully re-failed the test (though we did see a failure earlier w/o needing to remove the later logs in the test, but oh well). |
|
Attempting a 50x run of all tests except for the move tests since those time out. I think this might take a while. Will follow up with a rough estimate for completion if none fail. Edit: Approximately 65-70 seconds per run so I'm guessing about an hour to run through 50 times without failure. Edit 2: Though that's based on WebdriverIO's reported times. The runner actually took 2.5 minutes to fail so I think there's a lot of overhead not being reported here. That likely means this will take more than 2 hours to run through. I think GitHub has a 6 hour limit on runners so we should be below that. |
|
Interestingly running the whole suite actually caused the earlier test to fail again. Will try re-adding all of the logs and see if we can glean anything interesting. |
|
I've been able to partially isolate it. The move start test seems to consistently fail when run after the flyout/toolbox tests. None of the other suites seems to matter, but that one does. |
|
I've isolated it to the flyout tests opening an alert dialog and not closing it. For some reason the regular 'Start moving statement blocks' test needs to run and complete after the dialog has been opened in a previous test and before starting the 'Start moving value blocks' test begins in order to trigger the failure. |
|
I'm trying to reduce both of the move tests to just a single case rather than a loop and see if that still triggers the failure (in conjunction with the alert dialog). This is a rather bizarre situation and it's not at all clear to me what's happening since the tests should be largely isolated between runs. My best guess is that webdriver or Chrome is being put into a semi-broken state here. Edit: Isolating both move tests to 1 iteration seems to work. That means it's failing on the first iteration of the failing test and only 1 iteration is needed to get things into a broken state. |
This includes some dialog handling changes.
This reverts commit caf02c8.
|
Forcing session reloading fixes it. I may never fully understand the nuances that led to this particular issue, but I'm going to try reenabling the whole suite and also upping the timeout threshold for the Mac tests since there are a few that get close to 10s. We also are going to have a bit slowdown with this change since there's a lot of overhead in reloading the entire browser for every suite, but it should tremendously improve stability especially on Mac. |
|
Looks like most timeouts are already 30s so only needed to make a few adjustments there. |
|
Looks like everything is passing, and honestly the runtime isn't terrible. I'm going to try kicking off 50 runs and see how far both Linux & Mac get to see how stable they are now. |

Fixes #<issue_number_goes_here>